Fix haskellPackages.dhall_1_28_0 and spago by ijaketak · Pull Request #75931 · NixOS/nixpkgs

ijaketak · 2019-12-19T08:57:33Z

Motivation for this change

Fix #75930. See also purescript/spago#529

Things done

Notify maintainers

cc @

cdepillabout · 2019-12-19T09:14:16Z

pkgs/development/haskell-modules/configuration-nix.nix

Could you expand this comment so that the nixpkgs maintainers will have a better idea when these overrides can be removed (without having to follow the link)?

Done. Link was wrong.
dhall-lang/dhall-haskell@dedd5e0

cdepillabout · 2019-12-19T09:14:59Z

pkgs/development/tools/purescript/spago/default.nix

This file is generated with cabal2nix.

Could you instead add these patches to pkgs/developement/haskell/configuration-common.nix?

cdepillabout · 2019-12-19T09:17:15Z

pkgs/development/tools/purescript/spago/spago-0.12.1-dhall-1.28.0.patch

If possible, we generally don't like to carry around patches in the nixpkgs repo, but instead fetch them from upstream.

Could you pull this patch directly from upstream and apply it using fetchPatch. If you grep through pkgs/development/haskell-modules/configuration-common.nix you should be able to find examples of people pulling in patches from GitHub PRs.

purescript/spago#529 cannot be applied for spago-0.12.1. different patch required.

@ijaketak Please comment that on top of the patch.

cdepillabout

@ijaketak Thanks a lot for trying to fix up spago here.

If you take care of the above problems, we can merge this in.

Alternatively, it may just be a lot easier to create a dhall_1_27_0 derivation and setup spago so it uses it. See #75449 (comment) where I describe how to do this.

cdepillabout · 2019-12-19T15:05:35Z

@ijaketak Could you change this line to be dhall = dhall_1_27_0; (or dhall_1_28_0 depending on what you choose to do)?

https://github.com/NixOS/nixpkgs/pull/75931/files#diff-607a8b2e94748382228f44039b753cf0R660

I messed this up when I originally submitted the PR adding spago.

cdepillabout · 2019-12-19T15:35:32Z

@ijaketak I got spago building statically. I did it by just using dhall_1_27_0:

purescript/spago#528 (comment)

This was pretty easy. I recommend you do the same for this PR.

ijaketak · 2019-12-19T20:51:03Z

Let me conclude that this PR is completed as dhall_1_28_0 is fixed and spago is compatible with this version.

nh2

I found two places that should be self instead of super (so that they can be further overridden).

Beyond that and adding a comment for the reason of the patch, this change looks good to me.

nh2 · 2019-12-19T22:10:54Z

pkgs/development/haskell-modules/configuration-nix.nix

Suggested change

prettyprinter-ansi-terminal = super.prettyprinter-ansi-terminal.override { prettyprinter = super.prettyprinter_1_5_1; };

prettyprinter-ansi-terminal = super.prettyprinter-ansi-terminal.override { prettyprinter = self.prettyprinter_1_5_1; };

nh2 · 2019-12-19T22:11:40Z

pkgs/development/haskell-modules/configuration-nix.nix

Suggested change

prettyprinter = super.prettyprinter_1_5_1;

prettyprinter = self.prettyprinter_1_5_1;

cdepillabout · 2019-12-20T03:42:54Z

@ijaketak I'd like to bump spago to 0.13.0, since a new version was just released. Do you mind if I take over this PR for now?

I'll get dhall-1.28.0 building (basically using what you currently have here), as well as fix up spago to 0.13.0.

ijaketak · 2019-12-20T03:52:24Z

@cdepillabout No, please take over.

dhall-lang/dhall-haskell@dedd5e0 This raises the lower bound on prettyprinter to 1.5.1 since `removeTrailingWhitespace` is buggy in earlier versions.

cdepillabout · 2019-12-20T05:01:09Z

I sent #76079 using the stuff from this PR to get dhall_1_28_0 building.

I also sent #76084 updating to spago-0.13.0.

@ijaketak

This PR fixes dhall_1_28_0, dhall-bash_1_0_25, and dhall-json_1_6_0 so they build. They all require a newer version of prettyprinter than we get from the LTS package set. This is from #75931 by @ijaketak. Co-authored-by: Keito Kajitani <ijaketak@gmail.com>

@ijaketak

This PR fixes dhall_1_28_0, dhall-bash_1_0_25, and dhall-json_1_6_0 so they build. They all require a newer version of prettyprinter than we get from the LTS package set. This is from #75931 by @ijaketak. Co-authored-by: Keito Kajitani <ijaketak@gmail.com>

@ijaketak

This PR fixes dhall_1_28_0, dhall-bash_1_0_25, and dhall-json_1_6_0 so they build. They all require a newer version of prettyprinter than we get from the LTS package set. This is from #75931 by @ijaketak. Co-authored-by: Keito Kajitani <ijaketak@gmail.com>

ijaketak requested review from basvandijk and cdepillabout as code owners December 19, 2019 08:57

ofborg bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label Dec 19, 2019

cdepillabout reviewed Dec 19, 2019

View reviewed changes

ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Dec 19, 2019

cdepillabout reviewed Dec 19, 2019

View reviewed changes

cdepillabout requested changes Dec 19, 2019

View reviewed changes

nh2 reviewed Dec 19, 2019

View reviewed changes

ijaketak added 2 commits December 20, 2019 13:53

haskellPackages.dhall_1_28_0: depends on prettyprinter-1.5.1

669e9b5

dhall-lang/dhall-haskell@dedd5e0 This raises the lower bound on prettyprinter to 1.5.1 since `removeTrailingWhitespace` is buggy in earlier versions.

spago: port to dhall-1.28.0

24670ee

cdepillabout mentioned this pull request Dec 20, 2019

haskellPackages.dhall_1_28_0: fix overrides so it builds #76079

Merged

10 tasks

ijaketak closed this Dec 20, 2019

	prettyprinter-ansi-terminal = super.prettyprinter-ansi-terminal.override { prettyprinter = super.prettyprinter_1_5_1; };
	prettyprinter-ansi-terminal = super.prettyprinter-ansi-terminal.override { prettyprinter = self.prettyprinter_1_5_1; };

	prettyprinter = super.prettyprinter_1_5_1;
	prettyprinter = self.prettyprinter_1_5_1;

Uh oh!

Conversation

ijaketak commented Dec 19, 2019

Motivation for this change

Things done

Notify maintainers

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdepillabout left a comment

Choose a reason for hiding this comment

Uh oh!

cdepillabout commented Dec 19, 2019

Uh oh!

cdepillabout commented Dec 19, 2019

Uh oh!

ijaketak commented Dec 19, 2019

Uh oh!

nh2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdepillabout commented Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ijaketak commented Dec 20, 2019

Uh oh!

cdepillabout commented Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cdepillabout commented Dec 20, 2019 •

edited

Loading

cdepillabout commented Dec 20, 2019 •

edited

Loading